Use Instant and log dropped FFE flags#11535
Conversation
|
…2339-instant-allocation-windows # Conflicts: # products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java # products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
🐑 PR Shepherd is maintaining this PRI watch your PR and automatically fix CI failures, rebase your branch, handle flaky tests, and push it to the merge queue when it's ready. More about what I do → Guide To pause me on this PR, add the |
| private static Instant parseInstant(String value) { | ||
| return Instant.parse(value); | ||
| } |
There was a problem hiding this comment.
nitpick: I think parseInstant can be inlined as Instant.parse(value) is short enough.
| } else { | ||
| assertNotNull(parsed); | ||
| assertEquals(Instant.ofEpochMilli(expectedEpochMilli), parsed.toInstant()); | ||
| assertEquals(Instant.parse(expectedInstant), parsed); |
There was a problem hiding this comment.
note: There's also a Instans.ofEpochMilli
Another option could be change the assertion to compare the value directly rather than parsing the expected value as well, e.g. in pseudo code
| assertEquals(Instant.parse(expectedInstant), parsed); | |
| assertEquals(expectedEpochMilli, parsed.toEpochMilli()); |
This assertion could also work using parsed.toString().
Motivation
PR #11534 keeps
Dateat the UFC allocation boundary even though allocation windows are UTC instants. This follow-up removes the legacyDateconversion so payload parsing and evaluation preserve timestamp precision all the way through the allocation model.The same parser also drops malformed individual flags to keep valid flags evaluating. That recovery path should not be silent because an operator needs a breadcrumb when a flag is rejected locally.
This is now based on
masterafter #11534 merged.Changes
This changes the UFC v1
Allocationmodel to holdInstantvalues forstartAtandendAt. The remote config parser now registers anInstantadapter directly, and the OpenFeature evaluator checks allocation windows withInstant.now()plusisBeforeandisAfter, preserving the existing inclusive boundary behavior.When a flag cannot be parsed, the parser still skips only that flag, but now emits a warning with the flag key and parser error summary. The log does not include the flag payload.
The tests now parse expected instants directly and keep coverage for UTC offsets, invalid values, and active or inactive allocation windows.
Decisions
The change stays limited to feature flagging allocation windows and malformed flag parsing. It does not alter
ServerConfiguration.createdAt, which is still modeled as a string, or unrelated OpenFeature value type behavior. The adapter keepsISO_OFFSET_DATE_TIMEparsing instead ofInstant.parseso offsets such as+01:00and-05:00remain accepted.A warning log is used instead of metric plumbing because the deserializer path does not currently receive a metrics client, and adding one would widen construction for a narrow parser observability issue.
Validation
./gradlew :products:feature-flagging:feature-flagging-bootstrap:spotlessCheck :products:feature-flagging:feature-flagging-lib:test :products:feature-flagging:feature-flagging-lib:spotlessCheck :products:feature-flagging:feature-flagging-api:test :products:feature-flagging:feature-flagging-api:spotlessCheckBUILD SUCCESSFUL in 17s